Code Review

Overview

This is a focused, correct CSS fix: reverts mix-blend-mode from the per-tile img.leaflet-tile selector back to the .hillshade-blend container level, and adds isolation: isolate to #map to solve the underlying stacking-context problem. The technical reasoning in the PR description is sound.

What's working well

• Correct root-cause fix. Moving the blend to the layer container (peer of the base-layer container inside .leaflet-tile-pane) gives multiply a proper backdrop. The per-tile approach was blending against an empty stacking context — nothing to multiply against.
• isolation: isolate on #map addresses the original bug from fix(hillshade): mix-blend-mode sometimes not applied at closest zoom on topo layer #168, not just the regression. This is the right long-term fix.
• Small, surgical diff with zero risk of collateral breakage.

Issues

Comment style violates CLAUDE.md conventions

CLAUDE.md says:

"never write multi-paragraph docstrings or multi-line comment blocks — one short line max"
"Don't reference the current task, fix, or callers … since those belong in the PR description and rot as the codebase evolves."

Both new comments are multi-line blocks, and the second one explicitly references "PR #169", which will be meaningless context once this merges and time passes. The PR description already carries this history — the code comments should be trimmed to just the non-obvious WHY.

Suggested replacements:

/* isolation: isolate keeps mix-blend-mode contained to the map's own stacking context */
#map { position: absolute; top: 0; bottom: 0; right: 0; left: 0; isolation: isolate; }

/* blend at container level so .hillshade-blend composites against the base-layer sibling below it */
.hillshade-blend { mix-blend-mode: multiply; }

That's one short line each — captures the non-obvious WHY without the implementation history.

Manual test plan is incomplete

Three of the five manual test steps remain unchecked. These cover the core scenarios (hillshade visible without blocking basemap, no dropouts at max zoom, basemap switching). The fix should be manually verified before merge, especially since the previous regression (#171) was a visual failure that escaped without a browser test.

Minor / Non-blocking

• Performance of isolation: isolate: This promotes #map to a compositing layer, which is a tiny overhead. It's the right trade-off here since the map is already GPU-composited via Leaflet's translate3d tiles, but worth knowing if profiling ever flags paint costs.
• No type-check or lint impact (CSS-only change) — confirmed by the PR's test checklist.

Summary

The fix is technically correct and well-motivated. Two things before merging:

1. Trim the comments to single lines and remove the PR reference per CLAUDE.md conventions.
2. Complete the manual test checklist — hillshade + basemap switching is exactly the scenario that caught the last regression.

🤖 Generated with Claude Code

Code Review — PR #172

Overview

A minimal, targeted CSS-only fix (4 additions / 3 deletions) for the hillshade blend regression introduced in #169. Reverts mix-blend-mode from per-tile images back to the layer container, and adds isolation: isolate to #map to address the underlying stacking-context root cause from #168.

Correctness ✅

The CSS stacking-context reasoning is sound:

• Per-tile blend (broken): mix-blend-mode on .hillshade-blend img.leaflet-tile composites each tile against its parent .leaflet-tile-container. Leaflet uses transform: translate3d(…) on tile containers, which promotes them to their own compositing layer — a stacking context that contains only hillshade tiles. With no base-map pixels as backdrop, multiply renders the tiles opaque and hides everything beneath. This explains the reported "hillshade is all that is visible" symptom exactly.
• Container-level blend (correct): .hillshade-blend sits as a sibling of the base-layer container inside .leaflet-tile-pane. mix-blend-mode on the container composites against the base-layer container below it — the intended backdrop.
• isolation: isolate on #map: Prevents an ancestor compositing layer (e.g. a browser-promoted element outside #map) from absorbing the stacking context and disrupting the blend. This is the right place to create the boundary.

Code Quality ✅

• Change is minimal and scoped precisely to the problem.
• Both comments explain the why (non-obvious CSS compositing behavior) — consistent with the project's guidance to only comment when the reason would surprise a reader.
• No TypeScript, logic, or test changes needed; this is purely presentational.

Minor Observations

1. Comment verbosity (style note, non-blocking): The blend comment /* Blend at the layer container so multiply composites against the base-layer sibling below. */ is helpful for this tricky area, but it does partially describe what as well as why. The current wording is fine — just noting it for awareness against the project's "no what-comments" guideline. The isolation: isolate comment is spot-on.

2. Browser compatibility: isolation: isolate is broadly supported (Chrome 41+, Firefox 36+, Safari 8+). No concerns for a PWA targeting modern mobile browsers.

3. isolation: isolate z-index side effect: Creating a new stacking context on #map means z-index values on elements inside #map are no longer compared against elements outside it. Given #map fills the entire viewport with position: absolute, this is almost certainly a non-issue — but worth a quick sanity check that any overlays (modals, consent dialogs) rendered inside #map still stack correctly against each other.

Test Plan

The manual test checklist in the PR description is appropriate for a visual CSS change — automated unit tests can't cover blend-mode rendering. Suggested additions to the checklist:

• Hillshade disabled — base map renders normally, no visual regressions
• Consent modal / any overlay visible with Hillshade enabled — z-index stacking unaffected by isolation: isolate

Summary

This is a clean, well-reasoned fix. The PR description's stacking-context explanation is excellent — it makes the intent auditable in the future. Ready to merge once the manual rendering checks pass.

🤖 Generated with Claude Code